Repository navigation
Add addressed datagrams to the native socket runtime - #32
Conversation
284km
left a comment
284km
left a comment
There was a problem hiding this comment.
Reviewed 76023a6f against base f350170f. Found one Minor correctness issue in zero-capacity receives; see the inline comment.
The shared native-runtime implementation keeps C and LLVM behavior aligned. The subtraction-based arena bounds checks avoid overflowing an offset-plus-length calculation, and the loopback fixture checks source identity and preservation of the socket's binding and peer.
Validation on an isolated snapshot of the PR head, on macOS:
dune runtest: 2946 passed, 0 failed.datagram_check.sh: C and LLVM passed;sockaddr_check.sh: 116/116 passed.- Additional C/LLVM probes passed at
-O0and with UBSan at-O2: oversized descriptors, offsets, lengths and flags, empty sends, host-verifiedMSG_TRUNCcounts, and thread-local errno isolation. - The additional non-threaded probes also passed ASan/UBSan. ASan with the threaded probe was inconclusive because the sanitizer runtime failed to deallocate memory.
- CI configuration self-tests retained all 188 gates, including the new runtime gate, and passed the routing, result and setup negative controls.
The zero-capacity failure was reproduced on both C and LLVM. Linux execution was not repeated locally.
| " ssize_t n = recvfrom((int)fd, __mem + p, (size_t)cap, (int)flags, (struct sockaddr*)&ss, &sl);"; | ||
| " if (n < 0) return __sock_none(errno);"; | ||
| " char* address = __sock_text((struct sockaddr*)&ss, sl);"; | ||
| " if (!address[0]) return address;"; |
There was a problem hiding this comment.
[Minor] Handle zero-capacity receives before reporting a datagram
cap == 0 passes validation, but on macOS a zero-capacity recvfrom can return n == 0 with sl == 0 even when no packet has been sent. The cleared sockaddr then becomes "other 0 ", and this helper reports "0 other 0 " with fd_last_errno() == 0. Callers therefore see a successful receive with invalid sender metadata rather than a timeout/error.
Reproduced on both C and LLVM with the existing extern declarations:
let fd = socket 2 2 0;
let _ = sock_bind fd "127.0.0.1" 0;
let _ = tcp_set_timeout fd 100;
let p = mem_alloc 1;
print (sock_recvfrom fd p 0 0)
No sender is needed. Changing the capacity to 1 correctly returns "" with EAGAIN after the timeout.
Please either reject zero receive capacity with EINVAL and document the positive-capacity requirement, or handle zero-capacity receives so that success requires an actual packet and valid sender metadata. Add a no-packet regression and retain coverage for genuine empty datagrams received with positive capacity.
There was a problem hiding this comment.
Fixed in 7b101ac. sock_recvfrom now maps a zero-capacity result with no sender (sl == 0) to an empty result with EAGAIN, instead of constructing 0 other 0 . Zero capacity remains supported when the kernel actually supplies a packet and its sender.
Added IPv4/IPv6 regressions for no queued packet on timeout-configured and nonblocking sockets, plus queued empty and nonempty packets received at zero capacity. The latter checks verify sender identity, unchanged payload storage, and removal from the receive queue. Existing positive-capacity empty-datagram coverage remains in place.
The no-packet regression fails on the old implementation. The expanded gate passes on C and LLVM on macOS and C on Linux; the existing sockaddr gate passes all 116 checks.
| let _ = check "receive after peek" (str_eq (sock_recvfrom b q 16 0) (packet 3 aa)) in | ||
| let _ = check "receive binary" (str_eq (mem_to_str q 3) data && fd_last_errno () == 0) in | ||
| let _ = check "second destination" (sock_sendto a p 0 0 host (port ca) == 0) in | ||
| let _ = check "empty packet has sender" (str_eq (sock_recvfrom c q 16 0) (packet 0 aa)) in |
There was a problem hiding this comment.
Good regression coverage: sending to two destinations through the same bound socket and checking its binding and connected peer helps catch implementations that reconnect or replace the descriptor while adding addressed sends.
284km
left a comment
284km
left a comment
There was a problem hiding this comment.
Re-reviewed 7b101ac7, including the changes since 76023a6f. The previous Minor finding is addressed. No new actionable findings.
The missing-sender check prevents a zero-capacity receive from reporting a nonexistent packet. Queued empty and nonempty datagrams still return their actual sender, and the documentation now states the behavior explicitly. Polling before the queued-packet assertions avoids arrival races.
Validation on an isolated snapshot of the updated head, on macOS:
- The original reproducer now returns
""withEAGAINon both C and LLVM. - The updated datagram gate passed on C and LLVM for IPv4 and IPv6. Separate runs passed at
-O0and at-O2with ASan/UBSan. - Removing the new guard from generated C, and separately from the LLVM companion, made the new no-packet regression fail with the expected assertion. This confirms that the added coverage detects the original bug.
- Existing socket-address checks: 116/116 passed.
- CI configuration self-tests retained all 188 gates and passed their negative controls;
git diff --checkpassed.
GitHub CI still has queued/running jobs. This follow-up used focused runtime validation; the full unit suite and Linux execution were not repeated locally.
| " /* Darwin may return zero bytes and no sender for a zero-capacity"; | ||
| " receive with no queued datagram. Do not fabricate a successful"; | ||
| " packet; match the would-block/timeout result of a nonzero receive. */"; | ||
| " if (cap == 0 && sl == 0) return __sock_none(EAGAIN);"; |
There was a problem hiding this comment.
Verified: this resolves the previous zero-capacity finding on both native backends. Checking for missing sender metadata preserves successful zero-capacity receives of queued datagrams, while the original no-packet reproducer now returns an empty result with EAGAIN. The new regression also fails when this guard is removed.
284km
left a comment
284km
left a comment
There was a problem hiding this comment.
Follow-up review of 7b101ac7: no remaining actionable findings. The previous zero-capacity receive issue remains resolved. The code is unchanged since my last review, so the earlier local C/LLVM, sanitizer and negative-control results still apply.
I have now checked the completed CI results for this exact head:
- All 17 GitHub checks passed, including the four macOS/Ubuntu build configurations, both distribution compatibility jobs, and all nine gate groups.
- The Ubuntu 24.04 runtime job actually executed the new datagram gate: its log reports both
datagrams: c passedanddatagrams: llvm passed. The existing socket-address check also reports 116/116 passed. - The compiler gate job reports 2946 passed, 0 failed for the full unit suite.
The shared implementation and regression coverage now have successful native-backend validation on both macOS and Linux. CI verification is complete; no additional code changes are requested.
| run: sh scripts/sockaddr_check.sh | ||
| - name: datagram_check (addressed UDP on C and LLVM) | ||
| if: ${{ !cancelled() && matrix.group == 'runtime' }} | ||
| run: sh scripts/datagram_check.sh |
There was a problem hiding this comment.
Confirmed in the completed Ubuntu 24.04 runtime-job log: this step executed successfully and reports both C and LLVM passing. Together with the earlier macOS checks, this validates the shared runtime and the zero-capacity regression on both supported host platforms.
7b101ac to
991f299
Compare
|
Rebased onto Validation after rebasing: |
Addressed UDP currently has no native FFI, so mere-ruby cannot implement unconnected
UDPSocket#sendor obtain a datagram's sender. In its upstream DNS test, the sender raisesNotImplementedErrorand another thread waits until the 600-second file limit.Add
sock_sendtoandsock_recvfromto the shared C/LLVM native runtime. Destinations are numeric IPv4/IPv6 addresses; payloads use the byte arena, flags pass to the kernel, and failures preserve thread-local errno. Receive returns the syscall count together with the numeric sender address, including for empty datagrams. Both helpers validate arena bounds; callers must clamp aMSG_TRUNCcount before reading arena bytes. Sending preserves the existing descriptor, binding, and peer, including the host's refusal when Darwin rejects an addressed send on a connected socket. A zero-capacity receive without sender metadata returns EAGAIN; queued packets, including empty ones, still retain their sender and do not write to payload storage.Validation:
dune runtest: 2,946 passed, 0 failed.git diff --checkpass. The new gate runs in the runtime group, increasing the explicit inventory to 188 checks.The interpreter, Wasm, and RISC-V do not gain these capabilities. The runtime CI gate also checks C and LLVM on Linux.